Skip to content

fix(batch_evaluation): route batch_evaluation through v2 observations API - #1898

Open
passionworkeer wants to merge 7 commits into
langfuse:mainfrom
passionworkeer:oss/langfuse-python-1861-20260923T103452-nightly-5f51a956
Open

passionworkeer wants to merge 7 commits into
langfuse:mainfrom
passionworkeer:oss/langfuse-python-1861-20260923T103452-nightly-5f51a956

Conversation

@passionworkeer

@passionworkeer passionworkeer commented Sep 24, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes #1861 — batch_evaluation fails on Langfuse v4 events_only deployments because BatchEvaluationRunner._fetch_batch_with_retry still calls the v3 read endpoints (GET /api/public/traces and legacy GET /api/public/observations), both of which return:

This endpoint is not available on deployments running in Langfuse v4 events_only mode.

This PR routes both scope='traces' and scope='observations' through GET /api/public/v2/observations, which is the only read path that works on events_only deployments and is also the direction Langfuse is taking as v3 deprecates toward the 2026-11-16 EOL.

Implementation:

  • _fetch_batch_with_retry now calls client.api.observations.get_many with cursor/limit/filter/fields, replacing both the trace.list and the legacy.observations_v1.get_many calls. The page-mode page parameter is replaced with cursor pagination (response.meta.cursor).
  • A small helper _v2_observations_fields translates the legacy fetch_trace_fields argument into the v2 field-groups understood by /api/public/v2/observations. Caller-supplied groups are merged with the v2 defaults; legacy-only groups (observations, scores) are dropped because the v2 endpoint returns one observation at a time and does not expose trace-level sub-collections.
  • A second helper _collapse_observations_to_traces collapses a page of v2 observations to one representative per trace, preferring the one the server already marked as the root (is_root_observation=True) regardless of its position in the page.
  • scope='traces' requests are narrowed server-side to root observations, so the root choice is made once for the whole run rather than per cursor page. See "Root selection across cursor pages" below.
  • MapperFunction's Protocol union now includes ObservationV2 additively. The existing user-supplied mapper signature stays the same; mappers that read item.input / item.output / item.metadata work unchanged.
  • The pagination loop is rewritten around the cursor signal: iteration stops when the server returns meta.cursor=None.

Root selection across cursor pages

The v2 endpoint pages by cursor over observations, not traces, so one trace's root and its children can straddle a page boundary. Collapsing each page independently made the root preference best-effort exactly when it mattered: if the page carrying a child arrived first, the child became the representative, and the cross-page seen set then suppressed that trace on every later page — so the root was never evaluated at all.

scope='traces' therefore asks the server for roots by merging an isRootObservation = true condition into the v2 filter, which makes the representative a global choice rather than a per-page one. Two details that follow from the endpoint's documented behaviour:

  • The condition goes into the filter string, not the is_root_observation query parameter, because the endpoint documents that a supplied filter takes precedence over query-parameter filters — the query parameter would be silently ignored alongside a caller filter.
  • A caller condition on the same column is dropped rather than appended, since this code decides that column; a filter excluding roots would otherwise combine with the narrowing to return nothing.

The filter is not by itself a guarantee of one representative per trace, and both of the cases that break that assumption are covered by tests:

  • Multiple flagged roots on one trace. Two sibling spans on the same trace have both been observed carrying isRootObservation=true. What still holds is the collapse: it reduces a page to one observation per trace, and the cross-page seen set keeps it to one across pages, so such a trace is still evaluated exactly once.
  • No flagged root at all. A trace with no observation the server marks as a root is not returned, and so is not evaluated. is_root_observation is Optional[bool] on the model, and traces ingested by other clients — an OTel collector, the JS/TS SDK, direct OTLP — never pass through this SDK's app-root marking, so such a trace is reachable in practice, not theoretical. scope='traces' is one-observation-per-trace by definition, so evaluating an arbitrary descendant instead is not obviously better — but the drop is silent, which is why it is stated here.

A scope='traces' filter that is not a JSON array cannot be merged with the root condition, so run_async now rejects it before the fetch loop. Validating inside the loop would be swallowed by its except Exception into a completed=False result whose resume token carries an empty timestamp bound — a guaranteed 400 on the next run, with nothing pointing at the filter. scope='observations' forwards the caller's filter unchanged and is unaffected.

Behavioural change for scope='traces'

Items passed to the mapper go from TraceWithFullDetails — a whole trace, carrying the observations, scores and metrics arrays — to one ObservationV2 per trace, and only ever the root observation. Mappers that consumed the full observations / scores lists on a trace cannot be adapted field-for-field: those collections are gone rather than renamed. Two field-name traps follow from the item now being an observation:

  • item.id is the observation ID. The trace ID is item.trace_id, and that is also what score-create calls attach to (see _get_item_id).
  • item.user_id and item.tags do still work — the v2 observation carries them denormalized from the parent trace.

Mappers that read the root observation's input, output, metadata, model, usage work as-is. This change is the same direction the issue reporter suggested (workaround: "fetch v2 observations grouped by traceId, root observation for trace-level io") and aligns with langfuse/langfuse-python#1867 (batch evaluation marked as legacy, v2-only path).

scope='observations' items now also come from /api/public/v2/observations instead of the v1 read endpoint; items are ObservationV2 instances with input, output, metadata, model, usage and trace_context populated.

Known limitations (disclosed, not fixed here)

  • Metadata truncation: the generated client's docs say the v2 endpoint truncates metadata values to 200 characters by default, with expandMetadata returning full values. Measured on the self-hosted 4.42.0 events_only stack, a 300-character value came back untruncated both with and without expandMetadata, so the documented truncation was not observed there; behaviour may differ across server versions. The runner does not pass expandMetadata either way. If a deployment does truncate, passing a fetch_expand_metadata-style option through run_batched_evaluation would be the follow-up.
  • Score reads on events_only: both public score read endpoints are gated on events_only deployments — GET /api/public/scores and GET /api/public/v2/scores measured 404 on 4.42.0. Scores created by runs in this PR were verified by reading the events store (ClickHouse) directly; on Langfuse Cloud the UI is the read surface. (An earlier revision of this description said scores were visible at GET /api/public/v2/scores; that endpoint is not readable on this deployment today.)
  • scope='traces' resume boundary: resume filters on the representative observation's start_time with startTime > T. Narrowing to roots means that representative is now always the root, so the bound is the trace's own start rather than an arbitrary descendant's — a trace whose root started at or before T no longer re-enters a resumed run through a later child. A trace whose root started after T is of course still picked up, which is the intended behaviour. The residual gap is that T is the last processed root's start, so a trace that overlaps T can be evaluated twice across two runs; the v2 filter grammar has no not-equal operator on traceId and no trace-level read, so there is no clean fix at the API layer.
  • isRootObservation against a live server: verified on a self-hosted v4 events_only deployment at 621ccc83 — see the root-selection section below.

Type of change

  • Bug fix
  • New feature
  • Breaking change (item type for scope='traces'; see above)
  • Refactor
  • Documentation update
  • Tooling, CI, or repo maintenance

Verification

Commands run on 621ccc83 against 65392c73 (base_sha, current upstream main):

$ uv run --frozen pytest tests/unit/test_batch_evaluation_fetch.py -v
# 31 passed
#   (fetch layer: v2 routing, per-trace collapse incl. cross-page dedupe,
#    filter translation incl. id -> traceId, field-group translation,
#    root narrowing incl. caller-root-column replacement and per-page
#    re-derivation, scope-aware _get_item_id / _get_item_timestamp)
#   (run layer: max-items-on-last-page reporting, max-items mid-stream,
#    empty first page, _seen_trace_ids reset between runs,
#    flush before resume-token return, malformed-filter rejection)

$ uv run --frozen pytest -n auto --dist worksteal tests/unit
# 710 passed, 2 skipped, 58 warnings, 18 errors in tests/unit/test_prompt.py
# The 18 errors are pre-existing on base_sha (test_prompt.py requires
# LANGFUSE_PUBLIC_KEY / LANGFUSE_SECRET_KEY at fixture setup time); they are
# unrelated to this change. Verified by running that file on the unmodified
# base commit, which produces the same 18 errors.

$ uv run --frozen ruff check .
# All checks passed!

$ uv run --frozen ruff format --check langfuse/batch_evaluation.py tests/unit/test_batch_evaluation_fetch.py
# 2 files already formatted

$ uv run --frozen mypy langfuse --no-error-summary
# 0 errors

The six cases covering the root narrowing and the filter rejection were also run
against the previous implementation of _translate_trace_filter (the page-local
behaviour, without the run_async validation): all six fail there and pass on
131d296c/621ccc83, so they pin this change rather than restating it.

End-to-end against a real Langfuse v4.42.0 events_only docker-compose stack (postgres + clickhouse + redis + minio + langfuse-web + langfuse-worker):

  • BEFORE_FIX (65392c73): batch_evaluation returns 0 scores; every retry fails with HTTP 404 events_only on GET /api/public/v3/traces and the v3 observations endpoint.
  • AFTER_FIX (045f98aa): seeded 3 traces via SDK; ran batch_evaluation with scope='observations'; total_items_processed=22, total_scores_created=15, completed=true. New scores visible at GET /api/public/v2/scores.

Full end-to-end log: oss-agent/logs/20260923T103452-nightly-5f51a956/e2e-after-fix.log.

Root selection, measured on the same stack (621ccc83)

The deployment was confirmed events_only before any conclusion was drawn from
it: GET /api/public/traces and GET /api/public/observations return 404
events_only, while GET /api/public/v2/observations returns 200.

A trace with one root and twelve children, read back at page_size=2 so the
root cannot share a page with all of its children:

path representative picked evaluations of that trace
page-local collapse, no root filter (pre-fix) a child 1
same helper fed root-only pages (this change) the server-side root 1

The pre-fix path tracked the server's ordering — across repeated runs the root
was sometimes on page 1, when pre-fix also picked it, and sometimes not, when
pre-fix picked a child instead. The root-filtered path returned the root exactly
once in every run. That variance is why the e2e test records the observed
ordering instead of asserting a fixed one.

The filter was also checked in both directions, so it is not a no-op:
isRootObservation = true returns only the flagged root, and
isRootObservation <> true returns the twelve children and excludes it.

New file tests/e2e/test_batch_evaluation_root_selection.py (5 tests, passing)
covers all of the above: the filter being honoured in both directions, the root
winning across cursor pages, run_batched_evaluation(scope='traces') handing the
mapper the root's payload rather than a child's, and the two cases that break the
"one root per trace" assumption —

case what the server returned what the runner does
two sibling spans on one trace both flagged isRootObservation=true collapse still yields 1 representative; 1 evaluation across pages
span ingested over OTLP with a dangling parent none flagged filter returns nothing; the trace is not evaluated

The no-root case is written through the OTLP endpoint because the observation
write endpoints refuse creates on an events_only deployment:
POST /api/public/observations answers 405, and POST /api/public/ingestion
answers Event type "SPAN" is not accepted ... only accepts score events. That
error names the OTLP path as the events_only-compatible route, which is also the
realistic route for a trace ingested by another client.

The e2e file needs no shard registration — scripts/select_e2e_shard.py assigns
it to shard 0.

Remaining paths, re-verified on the same stack (621ccc83)

Everything else this PR touches was re-run end to end on the same deployment — a
4-trace corpus (root as a GENERATION carrying model, 2 child spans each, a
300-character metadata value on every root):

path check result
scope='observations' e2e 12 items, mapper saw input/output/metadata populated, model on every GENERATION, 12 scores created pass
score events (observations) in the events store: observation_id set on all 12, trace_id set pass
scope='traces' e2e 4 items, all roots, root's own io, 4 scores created pass
score attachment (traces) events store: observation_id empty on all 4, one per trace — attached to the trace, not the root observation pass
filter translation name v3 name= selected exactly the one named trace (via traceName) pass
filter translation id v3 id= selected exactly that trace (via traceId) pass
filter translation timestamp v3 timestamp > T selected exactly the corpus (via startTime) pass
observations filter pass-through a name filter stayed an observation-name filter: 1 item, the matching child pass
max_items mid-stream fetch_batch_size=1, max_items=1, 4 traces: processed 1, has_more_items=True pass
resume read path constructed token with startTime > T at the 2nd-earliest root: resumed run evaluated exactly the 2 later traces, no duplicates, completed=True pass
_additional_trace_tags tags did not land — events_only ingestion only accepts score events; the failure is logged client-side recorded

Two recorded observations that correct earlier statements in this description:

  • Metadata is not truncated on 4.42.0 — see the limitations section above.
  • A max_items stop reports completed=True with has_more_items=True and
    no resume token; only the fetch-failure path produces a token. A user
    wanting to resume a limit stop constructs the token from the result shape
    themselves. That is pre-existing semantics, unchanged by this PR, and worth
    knowing when reading has_more_items.

Scores were verified by querying the events store (ClickHouse) directly because
both public score read endpoints are gated on events_only deployments (see
limitations). Verification log with all thirteen checks: run
20261010T112000-morning-c4d5e6f7 in the private work repository.

Checklist

  • I self-reviewed the diff using code_review.md.
  • I added or updated tests for behavior changes (31 unit tests in tests/unit/test_batch_evaluation_fetch.py, 6 of them covering the root narrowing and the filter rejection, plus 5 e2e tests in tests/e2e/test_batch_evaluation_root_selection.py run against a live events_only deployment).
  • I updated docs, examples, or .env.template if needed — the run_async docstring and the module-level filter constants now describe the root narrowing; no user-facing config touched.
  • I did not hand-edit generated files; if generated files changed, I used the upstream regeneration path — langfuse/api/ was not touched. ObservationV2 is imported from the generated langfuse.api namespace.
  • I did not commit secrets or credentials.

Copilot AI lite review requested due to automatic review settings September 24, 2026 06:32

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant

CLAassistant commented Sep 24, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Resolve the metadata omission and cross-page trace collapsing issue; correct the inaccurate field-group comment.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Migrates batch_evaluation to the v2 observations API for Langfuse events_only deployments.

Changes:

  • Adds cursor-based v2 observation fetching and field mapping.
  • Collapses observations into trace representatives.
  • Adds unit coverage for fetching, grouping, pagination, and field selection.
File Summary Findings
tests/​unit/​test_batch_evaluation_fetch.py Tests v2 fetching, grouping, filtering, pagination, and field mapping. No findings.
langfuse/​batch_evaluation.py Implements v2 observation fetching and trace collapsing. Critical: default fields omit metadata. Moderate: trace collapsing is page-local and can duplicate traces. Nit: comment incorrectly describes supported field groups.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread langfuse/batch_evaluation.py Outdated
# The legacy ``/api/public/traces`` ``io`` / ``scores`` / ``observations`` /
# ``metrics`` field groups are not selectable on the v2 read endpoint because
# it returns one observation at a time.
_DEFAULT_V2_OBSERVATION_FIELDS = "core,basic,io,usage,model,trace_context"

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — metadata added to the default v2 field groups.

Comment thread langfuse/batch_evaluation.py Outdated
Comment on lines +63 to +72
def _collapse_observations_to_traces(
observations: List[ObservationV2],
) -> List[ObservationV2]:
"""Collapse a flat list of observations to one observation per trace_id.

The v2 observations endpoint has no trace-level read; this helper takes
whatever observations the page returned and returns one representative
per trace, preferring the observation that the server already marked as
the root (``is_root_observation=True``) and falling back to the first
observation seen for that trace.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — the runner now tracks seen trace IDs across cursor pages.

… API

BatchEvaluationRunner._fetch_batch_with_retry used the v3 read APIs
(GET /api/public/traces and GET /api/public/observations via legacy.observations_v1)
to fetch items for batch evaluation. Both endpoints return HTTP 400 on
Langfuse platform v4 events_only deployments, so every batch evaluation
against an event-store-backed self-hosted Langfuse v4 fails on the first
page (langfuse/langfuse#1861).

This change routes the runner through GET /api/public/v2/observations,
which is the only read path that works on v4 events_only (and remains
available on v3, with the v3 endpoint scheduled for removal on
2026-11-16). The pagination state switches from page-based to
cursor-based. For scope=traces, observations are collapsed to one
representative per trace (preferring is_root_observation=True), since
the v2 endpoint has no trace-level read.

The MapperFunction protocol accepts the v2 ObservationV2 in addition to
the legacy TraceWithFullDetails / ObservationsView, so existing mappers
keep working with the input/output/metadata fields they already read.
…e order

The v2 observations endpoint may not return observations with the trace root
first. The previous 2-dict implementation skipped the trace after its first
appearance, so a non-root observation could prevent the root from being
picked even when the root appeared later in the page.

Switch _collapse_observations_to_traces to a single-dict form that replaces
the chosen representative whenever a strictly better candidate appears.

Also expand unit tests:
  - test_fetch_batch_prefers_root_observation_regardless_of_page_order
    exercises the bug above.
  - four test_v2_observations_fields_* tests cover the
    fetch_trace_fields to v2 field-groups translation.

Cursor-loop comment is corrected to match the actual behaviour (the
implementation has always stopped on an empty page; the previous comment
about preserving the cursor on transient empty pages was wrong).
@passionworkeer
passionworkeer force-pushed the oss/langfuse-python-1861-20260923T103452-nightly-5f51a956 branch from 571f327 to 26eceed Compare September 24, 2026 06:40
Comment on lines +1276 to +1280
if scope == "traces":
items = cast(
List[Union[TraceWithFullDetails, ObservationsView, ObservationV2]],
_collapse_observations_to_traces(list(response.data)), # type: ignore[arg-type]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Observation IDs used as trace IDs When scope="traces", this path returns an ObservationV2, but the runner still uses item.id as the trace ID. That is the observation’s ID, not item.trace_id, so trace-level scores and optional tags are sent with the wrong trace ID and may be missing from the intended trace.

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1276-1280

Comment:
**Observation IDs used as trace IDs** When `scope="traces"`, this path returns an `ObservationV2`, but the runner still uses `item.id` as the trace ID. That is the observation’s ID, not `item.trace_id`, so trace-level scores and optional tags are sent with the wrong trace ID and may be missing from the intended trace.

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — _get_item_id returns trace_id for scope='traces'.

Comment thread langfuse/batch_evaluation.py Outdated
# The legacy ``/api/public/traces`` ``io`` / ``scores`` / ``observations`` /
# ``metrics`` field groups are not selectable on the v2 read endpoint because
# it returns one observation at a time.
_DEFAULT_V2_OBSERVATION_FIELDS = "core,basic,io,usage,model,trace_context"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Default fields omit metadata The v2 API requires a separate metadata field group; io supplies only input and output. With these defaults, observation metadata is absent, so a mapper that reads it receives None. The documented mapper’s observation.metadata.get(...) path then fails for non-generation observations.

Suggested change
_DEFAULT_V2_OBSERVATION_FIELDS = "core,basic,io,usage,model,trace_context"
_DEFAULT_V2_OBSERVATION_FIELDS = "core,basic,io,metadata,usage,model,trace_context"

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 41

Comment:
**Default fields omit metadata** The v2 API requires a separate `metadata` field group; `io` supplies only input and output. With these defaults, observation metadata is absent, so a mapper that reads it receives `None`. The documented mapper’s `observation.metadata.get(...)` path then fails for non-generation observations.

```suggestion
_DEFAULT_V2_OBSERVATION_FIELDS = "core,basic,io,metadata,usage,model,trace_context"
```

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — metadata added to the default v2 field groups.

Comment on lines +1276 to +1280
if scope == "traces":
items = cast(
List[Union[TraceWithFullDetails, ObservationsView, ObservationV2]],
_collapse_observations_to_traces(list(response.data)), # type: ignore[arg-type]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Traces repeat across pages If a trace’s observations span cursor pages, this helper chooses a representative independently on each page. The runner evaluates the trace again on the next page; a child on one page and the root on another can therefore produce separate evaluations for the same trace instead of one.

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1276-1280

Comment:
**Traces repeat across pages** If a trace’s observations span cursor pages, this helper chooses a representative independently on each page. The runner evaluates the trace again on the next page; a child on one page and the root on another can therefore produce separate evaluations for the same trace instead of one.

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — the runner now tracks seen trace IDs across cursor pages.

Comment on lines +1276 to +1280
if scope == "traces":
items = cast(
List[Union[TraceWithFullDetails, ObservationsView, ObservationV2]],
_collapse_observations_to_traces(list(response.data)), # type: ignore[arg-type]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Trace resumes lack a timestamp If a later page fails during scope="traces", the selected ObservationV2 has start_time but no timestamp. The runner consequently saves an empty resume timestamp, then applies a timestamp > "" filter to the v2 observations request. The resume token cannot reliably continue from the last processed item.

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1276-1280

Comment:
**Trace resumes lack a timestamp** If a later page fails during `scope="traces"`, the selected `ObservationV2` has `start_time` but no `timestamp`. The runner consequently saves an empty resume timestamp, then applies a `timestamp > ""` filter to the v2 observations request. The resume token cannot reliably continue from the last processed item.

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — uses the observation's start_time and the startTime filter column.

Comment on lines +1264 to +1270
response = self.client.api.observations.get_many( # type: ignore[union-attr]
cursor=cursor,
limit=limit,
filter=filter,
request_options={"max_retries": max_retries},
fields=v2_fields,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Trace filters change meaning For scope="traces", the original filter now goes to the observations endpoint unchanged. A name = "checkout" filter that previously selected traces by name now selects observations by name; a matching trace whose root observation has a different name is omitted, while an observation in an unrelated trace can match.

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1264-1270

Comment:
**Trace filters change meaning** For `scope="traces"`, the original filter now goes to the observations endpoint unchanged. A `name = "checkout"` filter that previously selected traces by name now selects observations by name; a matching trace whose root observation has a different name is omitted, while an observation in an unrelated trace can match.

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60, extended in 6865554 — name/timestamp/id translate to traceName/startTime/traceId.

Comment on lines +1169 to +1171
if max_items is not None and total_items_fetched >= max_items:
has_more = True # More items exist but we're stopping
break

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Final page reports more items When max_items is reached on a page whose v2 cursor is None, this branch sets has_more back to True. The result then reports has_more_items=True even though the server says there are no further results, which can prompt callers to look for work that does not exist.

Knowledge Base Used: Experiments and batch evaluation

Prompt To Fix With AI
This is a comment left during a code review.
Path: langfuse/batch_evaluation.py
Line: 1169-1171

Comment:
**Final page reports more items** When `max_items` is reached on a page whose v2 cursor is `None`, this branch sets `has_more` back to `True`. The result then reports `has_more_items=True` even though the server says there are no further results, which can prompt callers to look for work that does not exist.

**Knowledge Base Used:** [Experiments and batch evaluation](https://app.greptile.com/personal-org-4986/-/custom-context/knowledge-base/langfuse/langfuse-python/-/docs/experiments-and-batch-evaluation.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@passionworkeer passionworkeer Sep 24, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6021c60 — has_more stays false when the server already returned cursor=None.

Copilot and Greptile flagged six concerns against the previous two commits;
this commit resolves them in one place.

- Default v2 field groups: add ``metadata``. The ``io`` group only carries
  input/output strings; ``metadata`` is a separate v2 field group and was
  missing from the defaults, so every ``ObservationV2`` passed to a mapper
  had ``metadata=None``. The defaults are now
  ``core,basic,io,metadata,model,usage,trace_context``.
- Cross-page trace collapse state: ``BatchEvaluationRunner`` now tracks the
  trace IDs already collapsed on an earlier page in ``self._seen_trace_ids``
  and ``_collapse_observations_to_traces`` accepts an optional set so the
  same trace is never evaluated twice across cursor pages.
- ``_get_item_id`` for ``scope=traces``: the item is now an
  ``ObservationV2`` whose ``id`` is the observation ID, not the trace ID.
  Use ``trace_id`` instead so downstream score-create calls attach to the
  intended trace.
- ``_get_item_timestamp`` and ``_get_timestamp_field_for_scope``: use the
  observation ``start_time`` as a proxy for the trace timestamp; the v2
  filter column is ``startTime``. Resume tokens continue to work; legacy
  ``TraceWithFullDetails.timestamp`` is still consulted as a fallback.
- Final-page ``has_more`` report: do not set ``has_more = True`` when the
  server already returned ``cursor=None``; the previous code reported
  ``has_more_items=True`` even though the server said there were no further
  results.
- ``_translate_trace_filter``: rewrite ``name`` -> ``traceName`` and
  ``timestamp`` -> ``startTime`` in v3-shaped trace filters so a trace name
  filter selects the same set of traces on the v2 endpoint as it did on
  v3. The translation only runs for ``scope=traces``.

Tests: 6 new unit tests cover the cross-page collapse, filter translation,
the missing-metadata field group, the scope-aware ``_get_item_id`` and
``_get_item_timestamp`` paths, and the post-cursor-empty max-items path.
All 16 tests in tests/unit/test_batch_evaluation_fetch.py pass; the
larger unit suite shows 695 passed, 2 skipped, and the same 18 pre-existing
test_prompt.py credential errors that exist on base_sha.
@passionworkeer

passionworkeer commented Sep 24, 2026 •

Copy link
Copy Markdown
Author

cc @hassiebp for review when you have a moment.

Review feedback is addressed in 6021c60 and 6865554 (filter translation, trace-id usage, cross-page dedupe, resume timestamps, field defaults, has_more edge cases — details in the commit messages).

One open question: would you prefer the v2 read path as the default (v3 reads are deprecated with EOL 2026-11-16) or gated behind a flag? Happy to split a flag-gated rollout out as a follow-up if that's easier to land.

… paths

Follow-up to the review-fix commit, from a local pre-merge adversarial
review (12-item checklist run by a reviewer subagent).

Docstring corrections (no runtime change): the public docstrings for
run_batched_evaluation and BatchEvaluationRunner.run_async still claimed
the runner reads the v3 endpoints and "is not yet supported with platform
v4" — the exact opposite of what this PR does. They now describe the v2
observations endpoint, cursor pagination, the per-trace collapse, and the
v2 field-group semantics of fetch_trace_fields (legacy-only groups are
dropped; user-supplied groups are merged into the defaults).

Hardening:
- Trace filter translation now also rewrites ``id`` to ``traceId``; a v3
  trace-id filter previously matched against v2 observation ids and
  silently returned an empty selection.
- An empty page no longer leaves ``has_more`` set when the server also
  returned a cursor; the run is treated as exhausted, matching the
  pre-migration behaviour.
- The fetch-failure early return now flushes the client before building
  the resume token, so scores already created for earlier pages are not
  lost with the process (pre-existing gap on base, one-line fix).

Tests: five new run_async-level tests cover the max-items-on-last-page
reporting (completed=True / has_more_items=False), max-items mid-stream
(has_more_items=True), the empty-first-page completion, the
_seen_trace_ids reset between consecutive runs, and the flush before the
resume-token return. The filter-translation test now asserts the
``id -> traceId`` rewrite. 21 tests in the file; full unit suite 700
passed, 2 skipped, 18 pre-existing test_prompt.py credential errors.
The v2 observations endpoint pages by cursor over observations, not traces, so
one trace's root and its children can straddle a page boundary. The collapse
helper chose a representative within a page, so whichever page arrived first
fixed the representative for the whole run: if the child came first, the trace
was evaluated on the child and `_seen_trace_ids` then suppressed the root on
every later page. That made the root preference best-effort exactly when a
trace spans pages.

`scope='traces'` now narrows the request itself to root observations, so the
choice is made once for the whole run and each page yields at most one
observation per trace. The condition goes into the `filter` string rather than
the `is_root_observation` query parameter because the endpoint documents that
`filter` takes precedence over query-parameter filters; a caller condition on
the same column is dropped rather than appended, since a filter excluding
roots would otherwise combine with the narrowing to return nothing.

A `scope='traces'` filter that is not a JSON array cannot be merged with the
root condition, so it is now rejected in `run_async` before the fetch loop. The
fetch loop's `except Exception` would otherwise swallow it into a
`completed=False` result whose resume token carries an empty timestamp bound,
which is a guaranteed 400 on the next run with nothing pointing at the filter.
`scope='observations'` forwards the caller's filter unchanged and is unaffected.

Tests: the six new cases fail against the previous implementation and pass
against this one. `tests/unit/test_batch_evaluation_fetch.py` 31 passed,
`tests/unit` 710 passed / 2 skipped. The 18 errors in `tests/unit/test_prompt.py`
are environmental (no LANGFUSE_PUBLIC_KEY) and reproduce identically on the
unmodified base commit. ruff check, ruff format and mypy clean.

Not verified here: the `isRootObservation` filter against a live v4
events_only deployment, and the e2e suite.
@passionworkeer

Copy link
Copy Markdown
Author

Both review threads asked for root selection that is global to the run rather than per page, and that is now in 131d296c.

The cross-page dedup I added earlier only stopped a trace being evaluated twice; it did not fix which observation was chosen. If a page carrying a child arrived before the page carrying its root, the child became the representative and the seen set then suppressed that trace on every later page — so the root was never evaluated at all.

scope='traces' now asks the server for roots instead of choosing per page, by merging an isRootObservation = true condition into the v2 filter. With one root per trace, each page yields at most one observation per trace and page order stops mattering. Two details worth flagging:

  • The condition goes into the filter string rather than the is_root_observation query parameter, because the endpoint documents that a supplied filter takes precedence over query-parameter filters — the query parameter would be silently ignored whenever a caller filter is present.
  • A caller condition on the same column is dropped rather than appended, since this code decides that column. Appending would let a filter excluding roots combine with the narrowing to return nothing.

This also improves the resume boundary noted in the description: last_processed_timestamp is now always the root's start_time rather than an arbitrary descendant's, so a trace whose root started at or before T no longer re-enters a resumed run through a later child. The remaining overlap case is described in the updated limitations section.

Two costs, both stated in the description rather than left implicit:

  1. A trace with no observation the server marks as a root is not returned, so it is not evaluated. is_root_observation is Optional[bool] on the model, and traces ingested by other clients fall outside this SDK's app-root marking, so this is reachable.
  2. A scope='traces' filter that is not a JSON array now raises ValueError from run_async. The root condition has to be merged into that array, so it cannot be forwarded unchanged, and raising inside the fetch loop would be swallowed into a completed=False result whose resume token carries an empty timestamp bound — a guaranteed 400 next run with nothing pointing at the filter. scope='observations' is unaffected.

tests/unit/test_batch_evaluation_fetch.py is at 31 passed and tests/unit at 710 passed / 2 skipped; the 18 test_prompt.py errors are environmental and reproduce on the unmodified base commit. I checked the six new cases against the previous implementation and all six fail there, so they pin this change rather than restate it.

One gap I want to be explicit about: this verifies the filter shape against the generated client's documented schema, not against a live v4 events_only server. The e2e runs in the description predate this commit and cover the v4 read path, not isRootObservation. If reviewers want that covered before merge, I can stand up the events_only stack and add a multi-span trace case.

The unit tests pin the request the runner sends, but only a live deployment can
show that the isRootObservation filter is honoured, and that the cross-page bug
this replaced was real. Both are now asserted end to end in a new e2e file.

Measured against a self-hosted Langfuse 4.42.0 events_only stack (postgres +
clickhouse + redis + minio + web + worker), confirmed events_only first: the v3
read endpoints return 404 events_only while /api/public/v2/observations returns
200. A trace seeded with 12 children, read back at page_size 2, gave:

  pre-fix  (page-local collapse, no root filter): picked a child observation
  post-fix (same helper, root-only pages):        picked the server-side root

Repeated across runs the server ordering varied -- the root was sometimes on
page 1, sometimes not -- and the pre-fix path tracked that variation while the
post-fix path always returned the root exactly once. One earlier check had the
root on page 1, which is exactly the ordering that hides the bug, so the test
records that ordering rather than asserting a fixed one.

Also verified the filter is not a no-op in either direction: the equality
condition returns only the flagged root, and the negated condition returns the
children and excludes it.

The file needs no shard registration; select_e2e_shard.py assigns it to shard 0.
EOF
git log --oneline -2
@passionworkeer

Copy link
Copy Markdown
Author

Closing the gap I flagged in the previous comment: isRootObservation is now verified against a live v4 events_only deployment at c6c578e5, with three e2e tests added.

The deployment was confirmed events_only before anything was concluded from it — GET /api/public/traces and GET /api/public/observations return 404 events_only, GET /api/public/v2/observations returns 200.

The measurement that matters, on a trace with one root and twelve children read back at page_size=2:

path representative picked evaluations of that trace
page-local collapse, no root filter (pre-fix) a child 1
same helper fed root-only pages (this change) the server-side root 1

So the defect was real, not theoretical: with the root filter removed, the trace was evaluated on a child observation. What is also visible in the logs is that the pre-fix outcome depended on the server's row ordering — the root was sometimes on page 1, and pre-fix then picked it by luck. Across repeated runs of the new e2e test the ordering varied and the pre-fix path tracked that variation, while the root-filtered path returned the root every time. That is why the test records the observed ordering rather than asserting a fixed one; a test that pinned the lucky ordering would pass against the broken code.

I also checked the filter is not a no-op in either direction: isRootObservation = true returns only the flagged root, and isRootObservation <> true returns the twelve children and excludes it.

New file tests/e2e/test_batch_evaluation_root_selection.py (3 tests, passing) covers the filter being honoured, the root winning across cursor pages, and run_batched_evaluation(scope='traces') handing the mapper the root's payload rather than a child's. Per the repo's CI contract it needs no marker or registration — scripts/select_e2e_shard.py assigns it to shard 0.

One caveat on scope, unchanged from before: this verifies the filter and the root selection, not every other path in this PR. The v4 read path more broadly was verified against a real self-hosted deployment in an earlier revision, and the remaining limitation is unchanged — a trace with no observation the server marks as a root is not returned and therefore not evaluated.

…ong claim

Measuring the two edge cases the root narrowing rests on showed the PR body's
justification was wrong. It said "with one root per trace, every page yields at
most one observation per trace", but two sibling spans on the same trace were
both flagged isRootObservation=true by a real server. The filter does not
provide that guarantee.

What actually holds is the collapse: it reduces a page to one observation per
trace, and seen_trace_ids keeps it to one across pages. Corrected the comment in
batch_evaluation.py to say so, and to name the two cases that break the "one
root" assumption:

- Multiple flagged roots: two siblings on one trace, both flagged. Added a test
  asserting the collapse still yields one representative, and one evaluation
  across pages.
- No flagged root: a span ingested over OTLP with a parent id that does not
  exist never passes through this SDK's app-root marking, so nothing is flagged.
  Added a test asserting the filter returns nothing and scope='traces' never
  evaluates that trace -- the documented consequence, now pinned.

The observation write endpoints refuse creates on an events_only deployment
(/api/public/observations answers 405, /api/public/ingestion answers "Event type
not accepted ... only accepts score events"); the error names the OTLP path as
the events_only-compatible route, so that is what the no-root test uses.

Also corrected the run_async mapper example, which read metadata={"trace_id":
trace.id} -- for scope='traces' the item is an ObservationV2, so .id is the
observation ID while scores attach to trace_id.

Five e2e tests pass against the live events_only stack; unit suite 31 passed;
ruff and mypy clean.
@passionworkeer

Copy link
Copy Markdown
Author

Correcting something I got wrong in the previous two comments and in the PR description. I justified the root narrowing with "with one root per trace, every page yields at most one observation per trace". Measuring the edge cases on the live stack at 621ccc83 shows that is false: two sibling spans on the same trace were both flagged isRootObservation=true.

So the filter does not provide the one-representative guarantee. What actually provides it is the collapse — it reduces a page to one observation per trace, and the cross-page seen set keeps it to one across pages. The narrowing is still what makes the choice global rather than per-page; the count-per-trace property comes from those two, not from the filter. I have corrected the comment in batch_evaluation.py and the description accordingly.

Both cases that break the assumption are now covered:

case server returned runner behaviour
two sibling spans on one trace both flagged as root collapse still yields 1 representative; 1 evaluation across pages
span ingested over OTLP with a dangling parent none flagged filter returns nothing; the trace is not evaluated

The second row is the limitation I flagged before, now pinned by a test rather than only described. Producing it needed the OTLP endpoint: POST /api/public/observations answers 405 on this deployment and POST /api/public/ingestion answers Event type "SPAN" is not accepted ... only accepts score events. Both refusals name OTLP as the events_only-compatible write route, which is also the realistic way another client's trace arrives. The span is written with a parent id that does not exist, so there is no parent-less observation and no app-root marking — the same state a trace ingested by an OTel collector or the JS SDK lands in.

Also fixed a stale doc example: run_async's mapper sample read metadata={"trace_id": trace.id}, but for scope='traces' the item is an ObservationV2, so .id is the observation ID and scores attach to trace_id. The description now spells out that difference, and notes that item.user_id / item.tags still work because the v2 observation carries them denormalized from the parent trace.

tests/e2e/test_batch_evaluation_root_selection.py is now 5 tests, passing against the live stack; the unit suite is unchanged at 31 passed.

@passionworkeer

Copy link
Copy Markdown
Author

Re-ran every remaining path of this PR end to end on the same self-hosted 4.42.0 events_only stack, at the current head 621ccc83. Root selection was verified earlier; this covers the rest. All thirteen checks pass — the description now carries the full table. Highlights worth reading without opening it:

  • scope='observations' — 12 items, mapper saw input/output/metadata populated and model on every GENERATION observation, 12 score events created; in the events store all 12 carry observation_id and trace_id.
  • scope='traces' — 4 items, all roots, the root's own io, 4 score events whose observation_id is empty: attached to the trace, not the root observation. That pins the _get_item_id wiring through the real score-create path.
  • Filter translation against the live server — a v3-shaped name/id/timestamp filter each selected exactly the intended traces (traceName/traceId/startTime), and a scope='observations' name filter stayed an observation-name filter.
  • Resume — a token with startTime > T at the second-earliest root evaluated exactly the two later traces, no duplicates, completed=True.

Two measurements correct things this description previously claimed:

  1. Metadata is not truncated on 4.42.0. A 300-character metadata value came back untruncated, both with and without expandMetadata. The generated client's docs describe a 200-character default truncation, so this may be version-dependent — the limitations section now says exactly that instead of asserting truncation.
  2. The public score read endpoints are gated on events_only. GET /api/public/scores and GET /api/public/v2/scores both answer 404 with the events_only message on this deployment. An earlier revision of this description said scores were visible at GET /api/public/v2/scores; that is not readable here today. Scores for these runs were verified by querying the events store (ClickHouse) directly — on Cloud the UI is the read surface.

Two recorded observations, neither changed by this PR:

  • A max_items stop reports completed=True with has_more_items=True and no resume token — only the fetch-failure path produces one. Users resuming a limit stop construct the token themselves from the result shape.
  • _additional_trace_tags does not land on events_only: ingestion only accepts score events there, so the trace-create event is rejected and the failure is logged client-side. Pre-existing behaviour of the ingestion gate, now measured rather than assumed.

Corpus and commands: 4 traces, root as a GENERATION carrying model="gpt-verify", 2 child spans each, 300-char metadata on every root; runs issued through run_batched_evaluation with evaluators returning real Evaluations so score events actually flow. Nothing in these checks required code changes — the paths behave as the PR describes.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

batch_evaluation fails on self-hosted v4 events_only deployments (uses unavailable v3 read endpoints)

3 participants